Skip to content

fix(metadata)!: a $filter array that is not a filter AST is rejected, not passed through (#4121) - #4200

Merged
os-zhuang merged 1 commit into
mainfrom
claude/protocol-reject-malformed-filter
Jul 30, 2026
Merged

fix(metadata)!: a $filter array that is not a filter AST is rejected, not passed through (#4121)#4200
os-zhuang merged 1 commit into
mainfrom
claude/protocol-reject-malformed-filter

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #4121. Filed as item 2 of #3948error locality. The investigation found it is more than that.

It closes the last silently-unfiltered shape

#3948 made the drivers throw on a bare triple with an unknown operator, and on any element that is neither a join keyword nor a condition array. What it could not reach is a lone ['and'] / ['or']:

['and']  →  isFilterAST false   (a logical node needs length >= 2)
         →  assigned to `where` unconverted
         →  driver sets nextJoin, matches no element, emits NO predicate
         →  every row returned, silently

No driver-side check applies, because there is no offending element to reject — the array is well-formed and empty of conditions. It is the last survivor of the class #3948 closed, and it is now a 400.

For every other shape this is not a narrowing

That was the open question in #4121 — whether an opaque array where is a supported input. Traced through every driver:

Backend array where that is not an AST
driver-sql throws — applyFilters rejects any element that is neither and/or nor a condition array (#3948)
driver-memory throws (7 sites)
driver-mongodb its own parser, then a server error on the raw $${op}

So rejecting at the protocol changes which error the caller sees, not whether there is one. The gain is that the message is in the request's own vocabulary — unrecognised operator "not in", element 1 is number, plus the recognised operator list — instead of a driver's internal builder state, several layers from the query parameter that carried it.

Scoped narrowly, because the regression to fear is rejecting something valid

  • Only arrays are in scope. A where object is untouched: {status:'active'}, {$and:[…]}, {amount:{$gte:100}} all pass through exactly as before.
  • An empty [] is left alone. It means "no filter", and every path already treats it that way.
  • isFilterAST accepts nested arrays (filter.zod.ts"Legacy flat array: [[cond],[cond]]"), so [[a,'=',1],[b,'=',2]] and ['and', […], ['or', …]] keep converting. A naive "arrays are suspect" rule would have broken exactly those — which is why the accepted shapes are pinned by as many tests as the rejected ones.
  • before / after, which A filter with an operator outside VALID_AST_OPERATORS is silently dropped, not rejected — single-condition views return unfiltered results #3948 added to AST_OPERATOR_MAP, convert normally — verified, not assumed.

Errors carry status: 400 and code: 'INVALID_FILTER', matching the UNSUPPORTED_QUERY_PARAM convention immediately alongside.

Verification

  • 12 new tests driving the real findData normalisation — not a re-implementation of its rule. My first draft extracted the predicate into the test, which would have verified the rule while proving nothing about the wiring or the message; this drives ObjectStackProtocolImplementation.findData with a stub engine and asserts on the options handed to engine.find.
  • Six cover shapes that must keep converting, six the shapes that must be rejected — including exact message text and the status/code.
  • Confirmed the tests have teeth: disabling the new branch fails 6 of 12.
  • Full @objectstack/metadata-protocol suite: 122 tests across 19 files, green. tsc --noEmit: 0 errors in the touched files. eslint --no-inline-config: clean.

Refs #3948, objectstack-ai/objectui#2945

🤖 Generated with Claude Code

…4121)

isFilterAST was read as a CONVERSION gate: an array it refused was assigned
to options.where unconverted, leaving each backend to make sense of a value
the protocol had already decided it could not parse.

Filed as item 2 of #3948 — error locality. The investigation found more.

It closes the last silently-unfiltered shape. #3948 made the drivers throw
on a bare triple with an unknown operator and on any element that is neither
a join keyword nor a condition array. What it could not reach is a lone
['and'] / ['or']: the driver sets its join mode, matches no element, emits
NO predicate, and returns every row. isFilterAST refuses it (a logical node
needs length >= 2), so it arrived as an opaque `where` and no driver-side
check applied.

For every other shape this is not a narrowing: driver-sql throws,
driver-memory throws, driver-mongodb fails at the server. This changes WHICH
error the caller sees, not whether there is one — and puts the request's own
vocabulary in the message instead of a driver's builder state.

Scoped narrowly, because the regression to fear is rejecting something
valid:
- only arrays are in scope; a `where` OBJECT is untouched ($and/$or/$gte);
- an empty [] is left alone — it means "no filter";
- isFilterAST accepts nested arrays, so [[a,'=',1],[b,'=',2]] and
  ['and', […], ['or', …]] keep converting. A naive "arrays are suspect"
  rule would have broken those, so the accepted shapes are pinned by more
  tests than the rejected ones.

status 400 + code INVALID_FILTER, matching UNSUPPORTED_QUERY_PARAM alongside.

12 new tests driving the real findData normalisation rather than a
re-implementation of its rule; reverting the change fails six. Package suite
122 tests / 19 files green.

Closes #4121. Refs #3948, objectstack-ai/objectui#2945

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 30, 2026 3:04pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Jul 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

1 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A filter array that isn't a valid AST reaches the driver as an opaque where — reject it at the protocol instead

1 participant